-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
doc: add ref to option to enable n-api #13406
Conversation
Since its guarded in by a command line option say that in the docs and provide the option that needs to be used to enable it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
It might be better to add this a little further down, where #include <node_api.h>
is first introduced. Feel free to ignore if you want though.
doc/api/n-api.md
Outdated
@@ -11,6 +11,13 @@ changes in the underlying JavaScript engine and allow modules | |||
compiled for one version to run on later versions of Node.js without | |||
recompilation. | |||
|
|||
As the feature is currently experimental it must be enabled with the | |||
following command line option: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this link to https://nodejs.org/dist/latest-v8.x/docs/api/cli.html#cli_napi_modules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: remove currently
Pushed commit to address comments. |
GI good, landed as 7d9dfda. |
Since its guarded in by a command line option say that in the docs and provide the option that needs to be used to enable it. PR-URL: #13406 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießn <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Ohh cool, I'm an options now 😆 |
Since its guarded in by a command line option say that in the docs and provide the option that needs to be used to enable it. PR-URL: #13406 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießn <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Since its guarded in by a command line option say that in the docs and provide the option that needs to be used to enable it. PR-URL: nodejs#13406 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießn <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Since its guarded in by a command line option say that in the docs and provide the option that needs to be used to enable it. Backport-PR-URL: #19447 PR-URL: #13406 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießn <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Since its guarded in by a command line option say
that in the docs and provide the option that needs
to be used to enable it.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
doc, n-api